Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
daily-webapp	Ready	Preview	Jan 7, 2026 11:53am

1 Skipped Deployment

Project	Deployment	Review	Updated (UTC)
storybook	Ignored		Jan 7, 2026 11:53am

@idoshamun I tested it before it was indeed the safari crash loop on outdated sw, haven't validated though. it's just vibes that disable it all together for safari for now.

@claude please review and let me know if anything here can break the app?

Claude finished @rebelchris's task in 1m 43s —— View job

Code Review Progress

• Read modified files
• Analyze Safari detection logic
• Check for potential breaking changes
• Review service worker lifecycle handling
• Identify edge cases and race conditions
• Provide comprehensive feedback

Code Review Findings

I've analyzed the changes and identified several potential issues that could break the app or cause unexpected behavior:

⚠️ Critical Issues

1. Safari Detection Logic Mismatch Between Files (_app.tsx:156 vs sw.ts:18-21)

The Safari detection logic differs between the two files:

• _app.tsx uses getCurrentBrowserName() which checks if userAgent.includes('Safari') but doesn't filter out Chrome/Edge/etc properly
• sw.ts uses a custom regex that explicitly excludes Chrome|Chromium|Android|CriOS|FxiOS|Edg|OPR

This inconsistency is dangerous because:

• Chrome's UA contains "Safari" (e.g., Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36)
• If getCurrentBrowserName() incorrectly identifies Chrome as Safari, you'll disable service workers for all Chrome users

Let me check the actual Safari detection logic:

Looking at func.ts:126-154, the getCurrentBrowserName() function checks Chrome/Brave/Firefox/Edge before Safari, so it should work correctly. However, the order matters, and I'm concerned about the simple includes('Safari') check in UserAgent.Safari.

Verify Safari detection doesn't false-positive on Chrome →

2. Race Condition in Service Worker Unregistration (_app.tsx:124-154)

The unregisterSerwist() function runs asynchronously but isn't awaited:

if (browserName === BrowserName.Safari) {
  didRegisterSwRef.current = true;
  unregisterSerwist(); // ❌ Not awaited!
  return;
}

Why this is dangerous:

• The unregistration might still be in progress when the user navigates
• If a Safari user has an existing service worker, it might intercept requests before being unregistered
• This could lead to the exact WebKitInternal:0 crash you're trying to prevent

Fix: Either await the unregistration or move the logic outside the useEffect to ensure it completes before any navigation occurs.

Fix race condition by awaiting unregisterSerwist →

3. Service Worker clients.claim() Before Unregister (sw.ts:33-34)

await self.clients.claim();
await self.registration.unregister();

Problem: You call clients.claim() to take control of all clients, then immediately unregister. This creates a brief window where:

• The SW takes control of pages
• Then disappears
• Pages might be left in a controlled state with no active SW

This could cause navigation failures or cached responses to never load.

Better approach: Just skipWaiting() + unregister() without claiming clients.

Remove clients.claim() before unregister →

⚠️ Moderate Issues

4. Silent Error Swallowing (_app.tsx:151-153)

} catch {
  // Ignore cleanup failures – Safari sometimes rejects during shutdown.
}

While this prevents crashes, it also hides legitimate errors. Consider:

• Logging errors to analytics/monitoring (without crashing)
• Different handling for different error types

5. Missing Feature Flag or Gradual Rollout

This is a major behavioral change affecting all Safari users. If something goes wrong, you can't quickly disable it without deploying new code.

Recommendation: Wrap the Safari SW disabling logic in a feature flag (GrowthBook) so you can:

• Roll out gradually (e.g., 10% of Safari users first)
• Quickly disable if issues arise
• A/B test the impact

Add feature flag for Safari SW disabling →

6. No Cleanup of Existing Caches (_app.tsx:124-154)

You unregister the service worker, but don't clear the caches it created. This could leave:

• Stale cached resources that never update
• Orphaned cache storage taking up space

Consider adding cache cleanup:

if ('caches' in window) {
  const keys = await caches.keys();
  await Promise.all(keys.map(key => caches.delete(key)));
}

ℹ️ Minor Issues

7. Inconsistent didRegisterSwRef Usage (_app.tsx:86, 159, 168)

The ref is set to true for Safari (line 159) to prevent re-running, but this means if a user switches browsers (unlikely but possible in dev), the logic won't re-run. This is probably fine but worth noting.

8. Service Worker Detection Logic Incompleteness (_app.tsx:118-122)

The early returns don't account for:

• navigator.serviceWorker being defined but getRegistrations throwing (some privacy modes)
• The case where window.serwist is undefined but service workers are supported

These edge cases might leave some users without SW when they should have it (non-Safari browsers).

Summary

Will this break the app?

For non-Safari users: Probably not, assuming getCurrentBrowserName() correctly identifies Chrome/Edge/Firefox.

For Safari users: The race condition (#2) and clients.claim() issue (#3) could cause problems:

• Users might still hit the crash if unregistration doesn't complete fast enough
• Navigation might fail during the brief window where SW claims then unregisters

Biggest risks:

1. 🔴 Race condition - unregisterSerwist() not awaited
2. 🔴 Safari detection false positives - could disable SW for Chrome users
3. 🟡 No feature flag - can't roll back without deployment

Recommendations:

1. Fix the race condition by awaiting unregisterSerwist()
2. Add comprehensive logging to track Safari detection accuracy
3. Add a feature flag for gradual rollout
4. Test thoroughly on actual Safari devices (iOS checkbox still unchecked!)
5. Consider cache cleanup to prevent orphaned data

The core approach is sound, but the implementation has some rough edges that could cause issues. I'd recommend addressing issues #1-3 before merging.